Conversation
mprahl
force-pushed
the
ldap3-migration
branch
from
40a27bd to
8e80024
Compare
postmaster/ad.py
Outdated
| elif re.match(extract_username_regex, username): | ||
| # Parse the username from the UPN | ||
| username_search = re.search(extract_username_regex, username) | ||
| return self.domain + '\\' + username_search.group('username') |
There was a problem hiding this comment.
Either use .format or .join to format strings.
There was a problem hiding this comment.
Good point, I'll fix this tonight.
postmaster/ad.py
Outdated
| # If the authentication method is not NTLM, then distinguished names are valid usernames | ||
| return username | ||
| else: | ||
| return self.domain + '\\' + username |
There was a problem hiding this comment.
Either use .format or .join to format strings.
There was a problem hiding this comment.
Good point, I'll fix this tonight.
postmaster/utils.py
Outdated
| """ Validates the password from a wtforms object | ||
| """ | ||
| # Prevent circular import on json_logger by importing here | ||
| from postmaster.ad import AD, ADException |
There was a problem hiding this comment.
How does this create a circular import on json_logger?
There was a problem hiding this comment.
It's because AD relies on json_logger which is defined in utils.py. If I import AD at the top of the file, it tries to import json_logger which is not yet defined.
Is this okay or should we separate the files out somehow?
There was a problem hiding this comment.
We may run into this in the future. It may be better to separate the logger out into its own logger.py file.
| @@ -0,0 +1,912 @@ | |||
| { | |||
| "entries": [ | |||
There was a problem hiding this comment.
My oh my I hope this was exported from somewhere!
There was a problem hiding this comment.
Fortunately, yes :)
Here's an example from the docs:
from ldap3 import Server, Connection, ALL_ATTRIBUTES
server = Server('my_real_server')
connection = Connection(server, 'cn=my_real_user,ou=test,o=lab', 'my_real_password', auto_bind=True)
if connection.search('ou=test,o=lab', '(objectclass=*)', attributes=ALL_ATTRIBUTES):
connection.response_to_file('my_entries.json', raw=True)
|
Beautiful merge request. I can only really nitpick at one or two small things. The tests are good and the code is good. I really appreciate the hard work you put into that merge. I think the way we are handling config stuff in the database is getting a bit big. Controllable though, just something we may need to keep an eye on in the future. |
mprahl
force-pushed
the
ldap3-migration
branch
from
8e80024 to
dbc0950
Compare
|
@thatarchguy thoughts on the last commit? |
2 participants
This is a big PR, I think it'd be easier for you to review on a per commit basis (I tried to make those neat and concise changes).